Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wooki & 스타크] Kotlin Coroutine을 활용해서 동시에 여러 이미지를 다운로드 받아서 표시 및 PR 피드백 적용 #21

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

jminie-o8o
Copy link
Collaborator

@jminie-o8o jminie-o8o commented Mar 25, 2022

  • 이미지를 전부 다운로드할 때까지 기다리지않고, 이전 화면에서 + 버튼을 누르는 즉시 화면을 보여준다.
  • 다운로드를 받은 이미지를 아이템에 표시
  • 권한 두번 거절 시 스낵바에 "설정" 이라는 액션을 띄워주어 사용자가 클릭해야 설정창으로 이동하여 권한을 수정할 수 있도록 구현
  • CoordinateLayout 에서 layout_scrollFlags="scroll|enterAlways" 을 추가해 아래로 스크롤 시 AppBar 를 숨기고 다시 스크롤을 올리면 AppBar 가 보이도록 구현
  • GalleryAdapter, DoodlesAdapter 부분 findViewById 에서 뷰 바인딩으로 변경
2022-03-25.11.50.08.mov

Copy link
Contributor

@ivybae ivybae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jminie-o8o @banjjak2 수고하셨습니다~
코멘트 내용 확인해주세요~

import java.net.URL
import kotlin.system.measureTimeMillis

class ImageViewModel(application: Application) : AndroidViewModel(application) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jminie-o8o @banjjak2 ViewModelAndroidViewModel은 다른 클래스죠? 두 클래스의 차이는 무엇일까요? 저는 이 경우에는 ViewModel이 더 적합하다고 생각하고 있습니다

Copy link
Collaborator Author

@jminie-o8o jminie-o8o Mar 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@step4me

  • Application의 유무의 차이로써 AndroidViewModel 은 Application을 상속받아 Memory Leak이 발생할 수 있습니다.
  • Context와 직접 관련된 작업을 하는게 아니라면 공식문서에서도 ViewModel 을 권장하고 있다고 합니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asset 폴더에 접근하기 위해서 AndroidViewModel을 선택했는데 밑에 언급해주신대로 Json 파싱 부분을 데이터 레이아웃으로 빼게 된다면 ViewModel을 사용하는게 맞다고 생각합니다.

try {
val url = URL(image)
val stream: InputStream?
val time = measureTimeMillis {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jminie-o8o @banjjak2 measureTimeMillis은 무엇때문에 사용하게 되신거에요?

Copy link
Collaborator Author

@jminie-o8o jminie-o8o Mar 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@step4me
이미지 url을 openStream 시키는 시간을 측정해 사진을 불러오는 시간이 얼마나 되는지 보려고 했습니다. 테스트 용도라서 지웠어야 하는데 깜빡했습니다 ㅠ :(

_imageList.value = list
}

private fun getJsonDoodle() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jminie-o8o @banjjak2 이 구현은 data layer로 이동하는게 더 적합해 보이기도 하네요~ LocalDataSource에서 file을 읽은 데이터를 반환하면 되니까요~

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@step4me
네 알겠습니다!

getJsonDoodle()
jsonDoodleList.forEach {
val image = it.image
CoroutineScope(Dispatchers.IO).launch {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jminie-o8o @banjjak2 직접 구현해보신 점 좋습니다~ 다만 오늘 말씀드린것처럼 어느 시첨에 cancel 할지도 고민해보면 좋겠죠? 그리고 ViewModel 라이브러리를 추가하면 viewModelScope을 사용하시면 되고요!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어느 시점에 cancel할 것인지에 대해서 생각해봤는데 LifecycleObserver를 이용해 뷰의 onStop이나 onDestroy를 감시하다가 감지됐을 때 cancel할 수 있도록 구현해도 되는걸까요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants